-
Notifications
You must be signed in to change notification settings - Fork 0
[MBE]: Consolidate unspents for btc #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
mohammadalfaiyazbitgo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments to address and please rebase & regenerate the api json.
|
|
||
| const ConsolidateUnspentsResponse: HttpResponse = { | ||
| 200: t.any, | ||
| 202: t.any, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where do you return a 202?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
| }; | ||
|
|
||
| const ConsolidateUnspentsResponse: HttpResponse = { | ||
| 200: t.any, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please type this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
| }; | ||
|
|
||
| export const ConsolidateUnspentsRequest = { | ||
| pubkey: t.string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you have the walletId then you can just retrieve the pub key from the sdk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added the pubkey as a param in signing explicilty so they know they need to keep track of the pub. we still verify it against the pubkey on the wallet. can remove this in the future if we want
| pubkey: t.string, | ||
| source: t.union([t.literal('user'), t.literal('backup')]), | ||
| walletPassphrase: t.union([t.undefined, t.string]), | ||
| xprv: t.union([t.undefined, t.string]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mohammadalfaiyazbitgo mentioned only xprv removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
| const enclavedExpressClient = req.enclavedExpressClient; | ||
| const reqId = new RequestTracer(); | ||
| const bitgo = req.bitgo; | ||
| const baseCoin = bitgo.coin((req as any).params.coin); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use req.decoded it will be already typed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
| const params = (req as any).decoded; | ||
| const walletId = (req as any).params.walletId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to cast to any
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
|
|
||
| try { | ||
| // Create custom signing function that delegates to EBE | ||
| const customSigningFunction = async (signParams: any) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this need to be typed to any
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
a1a1bfc to
39e807a
Compare
| }; | ||
|
|
||
| export const ConsolidateUnspentsRequest = { | ||
| pubkey: t.string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added the pubkey as a param in signing explicilty so they know they need to keep track of the pub. we still verify it against the pubkey on the wallet. can remove this in the future if we want
| export const ConsolidateUnspentsRequest = { | ||
| pubkey: t.string, | ||
| source: t.union([t.literal('user'), t.literal('backup')]), | ||
| walletPassphrase: t.union([t.undefined, t.string]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this please, we're not using wallet passhphrases with onprem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
1fb05c1 to
2f9826f
Compare
6b184dd to
a910f96
Compare
a910f96 to
620f4d6
Compare
620f4d6 to
58ae73d
Compare
The base branch was changed.
feat(mbp): swagger update
1715fbb to
52e6a5a
Compare
[MBE]: Consolidate unspents for btc
Tasks: WP-4675